Hi. Thanks for your contribution. Can you please provide traintracks.xml for testing and describe expected behavior, so we can test it too?

Hi! Thank you for the response.

This resource, which I linked in the main PR message, can be used to test the changes.

Testing Steps

1. In the F8 menu, run the command track_count. This should return the message "Track count: 29".
2. In the F8 menu, run the command track_at_index 0. This should return the message "Track name hash at index 0: 3534397256".
3. In the F8 menu, run the command track_at_index 51. This should return the message "Track name hash at index 51: 0".
4. In the F8 menu, run the command load_tracks. This should return the following message:

Track count before load: 29
Track count after load: 34

5. In the F8 menu, run the command track_count. This should return the message "Track count: 34".
6. In the F8 menu, run the command spawn_trains. The game should not crash.
7. Exit the game, and navigate to traintracks.xml, which is located at <resources>/tracktest/data/. Open traintracks.xml in a text editor, and remove the contents of the <train_tracks> XML element. Save the file, and re-enter the game. In the F8 menu, run the command load_tracks. This should return the following message:

Track count before load: 29
Track count after load: 0

8. In the F8 menu, run the command spawn_trains. This should return the message "CreateMissionTrain() failed - track pool is empty!".
9. Repeat Step 7, but instead of removing the contents of the <train_tracks> element, copy one of the <train_track> elements within it and paste it multiple times - enough to push the number of <train_track> elements within the <train_tracks> node over 50. Save the file, re-enter the game, and in the F8 menu, run the command load_tracks. This should return the following message:

Track count before load: 29
Track pool is full - no more tracks can be loaded.
Track count after load: 50

Please let me know if I can provide additional information. Thank you!

I have updated my testing steps to include steps for testing having 0 train tracks in traintracks.xml, and having more than 50 tracks in it.

Hey, just checking in. Is there anything I can do to help the review process along?

Hey @Sage-of-Mirrors,
Checked the code with your testing steps and it works as intended.
My only concern is reloading the tracks when trains are already spawned leads to game crash.
Other than that, its ok from me.

Hey, thank you for the review!

I am aware of that crash; I did not address it in my original commit because it should only crash the client that calls LOAD_TRACKS_FROM_FILE. Would you like me to add a check in that native to prevent a reload if there are any trains in the level?

Hey, thank you for the review!

I am aware of that crash; I did not address it in my original commit because it should only crash the client that calls LOAD_TRACKS_FROM_FILE. Would you like me to add a check in that native to prevent a reload if there are any trains in the level?

Yes please, at least people will know something went wrong when the new api is misused.

While I was discussing this with @outsider31000, I was reminded that I had a different approach to this problem that involved an fxmanifest statement rather than natives: #2432. The PR was rejected by a previous member of the team.

Which method does Cfx feel is best? An fxmanifest statement seems much safer than a native that can be called at any point, since the statement would only be run at resource init. It would also get rid of the crashing-when-trains-exist issue altogether, since the reload only happens once, before any trains have been spawned.

Yeah I cant see any use case to reload tracks In runtime, even if there was one, it would still need to do it for all clients, doesnt seem a viable thing to do/allow it?
It should kinda be considered like a "ymap" load it once.
But that's just my opinion.

While I was discussing this with @outsider31000, I was reminded that I had a different approach to this problem that involved an fxmanifest statement rather than natives: #2432. The PR was rejected by a previous member of the team.

Which method does Cfx feel is best? An fxmanifest statement seems much safer than a native that can be called at any point, since the statement would only be run at resource init. It would also get rid of the crashing-when-trains-exist issue altogether, since the reload only happens once, before any trains have been spawned.

Just to be honest I liked previous solution more too. We need to think about it and maybe previous way will fit community needs better and in the same moment will be more stable

Of course the optimal solution is to make it configurable on server side and load it once on connect, while checking for multiple loaded track files to avoid collision. I thought this was designed with runtime editing in mind.

Of course the optimal solution is to make it configurable on server side and load it once on connect, while checking for multiple loaded track files to avoid collision. I thought this was designed with runtime editing in mind.

This native approach was suggested by disquse, who iirc felt that a manifest statement was not congruent with RedM's design principles and would be made obsolete when replacing the level meta became possible in RedM.

Perhaps a compromise can be reached: there is already a datafile loader for track data, TRAINTRACK_FILE. The original game code for the track loader does not attempt to dispose of the current track pool, it just adds the tracks to the existing pool and re-runs the post-processing functions on the whole pool. What if, instead of natives or a dedicated manifest statement, I modified the train file loader to dispose of the current track pool before loading the XML that was passed in? That way, a resource could just use datafile TRAINTRACK_FILE '<resource path>/data/traintracks.xml, and the tracks in the given file would replace the pool instead of add to what's already there.

Maybe there could be a server config boolean to explicitly turn on this behavior?

The crux of the issue here, however, is that the track resources have to be distributed to the client.

Another idea: maybe a replicated fxserver convar that the client checks for before loading the default track data? traintrack_override_xml or something. Server owner would still need to package the track data as a resource to distribute it, though.

I would say that the best approach would be to keep it within the resource loading the tracks. A convar for this instance would not be ideal compared to using resource metadata in the fxmanifest.

I have quite a bit of free time, so I will update the fxmanifest approach and add my crash patches to it.

Let me know what Cfx decides; I'm happy to finalize either method.

Here's the manifest version of this functionality: master...Sage-of-Mirrors:fivem:train-manifest-statement. This is what a resource using it would look like:

fx_version "cerulean"
game "rdr3"

rdr3_warning "I acknowledge that this is a prerelease build of RedM, and I am aware my resources *will* become incompatible once RedM ships."

files {
  'data/traintracks.xml',
  'data/greatplains_jnctw_rdr1.dat',
  'data/greatplains_main_rdr1.dat',
  'data/mexico_jncte_rdr1.dat',
  'data/mexico_jnctw_rdr1.dat',
  'data/mexico_main_rdr1.dat',
  'data/trains_old_west_intersection01.dat',
  'data/trains_old_west_intersection02.dat',
  'data/trains_old_west01.dat',
  'data/trains_old_west02.dat',
  'data/trains_old_west03.dat',
}

replace_traintrack_file 'data/traintracks.xml'

If this is a better solution than the one presented in this PR, I can close it and make another for the manifest version.

Here's the manifest version of this functionality: master...Sage-of-Mirrors:fivem:train-manifest-statement. This is what a resource using it would look like:

fx_version "cerulean"
game "rdr3"

rdr3_warning "I acknowledge that this is a prerelease build of RedM, and I am aware my resources *will* become incompatible once RedM ships."

files {
  'data/traintracks.xml',
  'data/greatplains_jnctw_rdr1.dat',
  'data/greatplains_main_rdr1.dat',
  'data/mexico_jncte_rdr1.dat',
  'data/mexico_jnctw_rdr1.dat',
  'data/mexico_main_rdr1.dat',
  'data/trains_old_west_intersection01.dat',
  'data/trains_old_west_intersection02.dat',
  'data/trains_old_west01.dat',
  'data/trains_old_west02.dat',
  'data/trains_old_west03.dat',
}

replace_traintrack_file 'data/traintracks.xml'

If this is a better solution than the one presented in this PR, I can close it and make another for the manifest version.

@Sage-of-Mirrors
Yes, that looks much better.
@prikolium-cfx

Any updates on which approach to this problem is best?